xen: sched: fix killing an uninitialized timer in free_pdata.
authorDario Faggioli <dario.faggioli@citrix.com>
Tue, 3 May 2016 21:46:42 +0000 (23:46 +0200)
committerGeorge Dunlap <george.dunlap@citrix.com>
Wed, 4 May 2016 16:42:07 +0000 (17:42 +0100)
commitf45666afdb01615feccde1a166042ce11befe751
tree0ebea005ff579fa3c6f725dfc8a798c11c5eabea
parent7dec5b0c658bea9c16a0e3c051e64d2abf57be48
xen: sched: fix killing an uninitialized timer in free_pdata.

commit 64269d9365 "sched: implement .init_pdata in Credit,
Credit2 and RTDS" helped fixing Credit2 runqueues, and
the races we had in switching scheduler for pCPUs, but
introduced another issue. In fact, if CPU bringup fails
during __cpu_up() (and, more precisely, after CPU_UP_PREPARE,
but before CPU_STARTING) the CPU_UP_CANCELED notifier
would be executed, which calls the free_pdata hook.

Such hook does, right now, two things: (1) undo the
initialization done inside the init_pdata hook and (2)
free the memory allocated by the alloc_pdata hook.

However, in the failure path just described, it is possible
that only alloc_pdata were called, and this is potentially
an issue (depending on how actually free_pdata does).

In fact, for Credit1 (the only scheduler that actually
allocates per-pCPU data), this result in calling kill_timer()
on a timer that had not yet been initialized, which causes
the following:

(XEN) Xen call trace:
(XEN)    [<000000000022e304>] timer.c#active_timer+0x8/0x24 (PC)
(XEN)    [<000000000022f624>] kill_timer+0x108/0x2e0 (LR)
(XEN)    [<00000000002208c0>] sched_credit.c#csched_free_pdata+0xd8/0x114
(XEN)    [<0000000000227a18>] schedule.c#cpu_schedule_callback+0xc0/0x12c
(XEN)    [<0000000000219944>] notifier_call_chain+0x78/0x9c
(XEN)    [<00000000002015fc>] cpu_up+0x104/0x130
(XEN)    [<000000000028f7c0>] start_xen+0xaf8/0xce0
(XEN)    [<00000000810021d8>] 00000000810021d8
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Assertion 'timer->status >= TIMER_STATUS_inactive' failed at timer.c:279
(XEN) ****************************************

Solve this by making the scheduler hooks API symmetric again,
i.e., by adding a deinit_pdata hook and making it responsible
of undoing what init_pdata did, rather than asking to free_pdata
to do everything.

This is cleaner and, in the case at hand, makes it possible to
only call free_pdata (which is the right thing to do) as only
allocation and no initialization was performed.

Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
xen/common/sched_credit.c
xen/common/sched_credit2.c
xen/common/schedule.c
xen/include/xen/sched-if.h